-
Notifications
You must be signed in to change notification settings - Fork 79
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(block): remove chat and messages when blocking a contact #16889
base: master
Are you sure you want to change the base?
Conversation
Jenkins Builds
|
singletonInstance.globalEvents.showContactRemoved("Contact removed", fmt "You removed {contact.displayName} as a contact", contact.id) | ||
signal = SIGNAL_CONTACT_REMOVED | ||
if contact.removed and not self.contacts[publicKey].dto.removed: | ||
singletonInstance.globalEvents.showContactRemoved("Contact removed", fmt "You removed {contact.displayName} as a contact", contact.id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to do the notification here at all; we do those (in the correct format) in Popups.qml already. There are different permutations, depending on what other actions the user takes via the popups (like marking as untrosted, blocking, etc). All of these are processed at once, so that you don't get a single notification for each action.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also (not a big deal atm) this is not translatable at all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's true. I guess we have a couple of stray notifications in Nim. I don't know if we have already an issue to refactor those to move them to QML
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, there was some initial effort by Noelia (see ToastsManager.qml) but never finished actually; I guess here's one of those (many) still done on the NIM side
Fixes #16640 This makes it so that when you block a contact, it now also removes the chat and the messages as expected by the requirements and as Mobile does. To do so, I use the same API as mobile instead of the forked desktop one. I removed the desktop one as it is no longer needed (see status-go PR) I also fixed an issue when unblocking where it would send a double toast messages with one saying you "removed the contact", but it was already removed.
487ffca
to
9400674
Compare
@caybro @igor-sirotin @iurimatias friendly reminder to review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I'll have a look at the notification within the other issue assigned to me
What does the PR do
Fixes #16640
This makes it so that when you block a contact, it now also removes the chat and the messages as expected by the requirements and as Mobile does.
To do so, I use the same API as mobile instead of the forked desktop one. I removed the desktop one as it is no longer needed (see status-go PR)
I also fixed an issue when unblocking where it would send a double toast messages with one saying you "removed the contact", but it was already removed.
Affected areas
Chat and contacts
Architecture compliance
My PR is consistent with this document: Status Desktop Architecture Guide
Screenshot of functionality (including design for comparison)
block-user.webm
Impact on end user
Removes the chat and message when blocking
How to test
Risk
Tick one:
Not much. Worst case some parts of the blocked user still appear until a restart.